Optimize Android emulator initialization by batching reboots by jardondiego · Pull Request #5280 · google/clusterfuzz

jardondiego · 2026-05-16T16:02:31Z

This change reduces the number of reboots during Android device setup by:

Adding a wait_for_reboot parameter to adb.write_data_to_file.
Tracking reboot status in initialize_device to skip the final reboot if one already occurred.
Disabling reboots when setting sanitizer options since the app restart is sufficient.

These optimizations improve bot startup efficiency and overall fuzzing throughput.

This change reduces the number of reboots during Android device setup by: - Adding a wait_for_reboot parameter to adb.write_data_to_file. - Tracking reboot status in initialize_device to skip the final reboot if one already occurred. - Disabling reboots when setting sanitizer options since the app restart is sufficient. These optimizations improve bot startup efficiency and overall fuzzing throughput.

… tracking and short-circuit issues

IvanBM18 · 2026-05-18T18:16:31Z

Nit:
Add typing hints and update the string doc for this method so that other devs reading this know what the bool means

IvanBM18 · 2026-05-18T18:17:21Z

+  return True
+

 def get_debug_props_and_values():


Nit x2:
Add typing hints and update the string doc for this method so that other devs reading this know what the bool means

Agree with Ivan

IvanBM18 · 2026-05-18T18:17:55Z

+      sanitizer_options, sanitizer_options_file_path, wait_for_reboot=False)


 def setup_asan_if_needed():


Nit x3:
Add typing hints and update the string doc for this method so that other devs reading this know what the bool means

IvanBM18

LGTM, minus some NITs

fernandofloresg

lgtm, would be nice to add those docstrings

fernandofloresg · 2026-05-18T22:27:32Z

+  return True
+

 def get_debug_props_and_values():


Agree with Ivan

Adds `-> bool` return type hints and explanatory docstrings to: - `configure_system_build_properties` - `setup_asan_if_needed` These updates clarify that the returned boolean indicates whether a device reboot occurred during the setup step.

letitz

I'm a bit confused about the main logic change in initialize_device(), see comment below.

letitz · 2026-05-19T08:28:05Z



-def write_data_to_file(contents, file_path):
+def write_data_to_file(contents, file_path, wait_for_reboot=True):


We should document the new argument at least. While we're at it, we can document the whole function: args, return value, any exceptions it raises.

letitz · 2026-05-19T08:30:07Z

  current_md5 = adb.get_file_checksum(BUILD_PROP_PATH)
  persistent_cache.set_value(constants.BUILD_PROP_MD5_KEY, current_md5)

+  return True


If I'm reading this right, we only rebooted on line 239 if the android version was as least M?

letitz · 2026-05-19T08:30:46Z


  # General device configuration settings.
-  configure_system_build_properties()
+  needs_full_reboot = configure_system_build_properties()


The docstring above states that this function returns whether the device was already rebooted, not whether it needed to be rebooted. Which is it?

letitz · 2026-05-19T08:32:48Z

  # Reboot device as above steps would need it and also it brings device in a
  # good state.
-  reboot()
+  if needs_full_reboot or not asan_reboot_done:


I'm having a hard time understanding the logic here, regardless of whether this boolean really means needs_full_reboot or if it means already_rebooted, per the above comment.

In the needs_full_reboot case: if we needed to reboot after configuring the device, and the asan setup step already rebooted it, then why do we reboot once more here? In other words, why is this not:

Suggested change

if needs_full_reboot or not asan_reboot_done:

if needs_full_reboot and not asan_reboot_done:

In the already_rebooted case: why do avoid rebooting if already_rebooted is true? I would expect instead:

Suggested change

if needs_full_reboot or not asan_reboot_done:

if not already_rebooted and not asan_reboot_done:

letitz · 2026-05-19T08:35:10Z

    return

-  adb.write_data_to_file(sanitizer_options, sanitizer_options_file_path)
+  # Skip reboot as the app will pick up the options file on restart.


Just to make sure I understand, this means that the next time the app is started, it will read from the options file?

letitz · 2026-05-19T08:42:31Z

+
+  def setUp(self):
+    # Mock environment to bypass the engine fuzzer check
+    mock.patch(


This is the first time I see mocks written like this in ClusterFuzz. Historically I've seen use of helpers.patch 1 and mock.patch decorators 2. In this case, I would expect the former.

letitz · 2026-05-19T08:43:45Z

+    self.mock_setup_asan.return_value = False
+
+    device.initialize_device()
+    self.mock_reboot.assert_called_once()


We test False/True, True/False and False/False. What about True/True? This goes back to my comment about or not vs and not.

letitz · 2026-05-19T08:44:33Z

+    mock.patch.stopall()
+
+  def test_reboot_if_props_changed(self):
+    """Test that it reboots if build.prop changed, even if ASan didn't run."""


Suggested change

"""Test that it reboots if build.prop changed, even if ASan didn't run."""

"""Test that `initialize_device()` reboots the device if

`configure_system_build_properties()` did, and the ASan setup script did not.

letitz · 2026-05-19T08:46:04Z

+    self.mock_reboot.assert_called_once()
+
+  def test_no_reboot_if_asan_ran(self):
+    """Test that the final reboot is skipped if ASan setup performed a shell restart."""


Suggested change

"""Test that the final reboot is skipped if ASan setup performed a shell restart."""

"""Test that `initialize_device()` skips calling `reboot()` if

`configure_system_build_properties()` did not cause a reboot but the ASan

setup script did.

"""

letitz · 2026-05-19T08:53:24Z

+    self.mock_reboot.assert_not_called()
+
+  def test_reboot_if_clean_slate_needed(self):
+    """Test that it still reboots to ensure a clean state if no other steps restarted it."""


Suggested change

"""Test that it still reboots to ensure a clean state if no other steps restarted it."""

"""Test that `initialize_device()` calls `reboot()` if neither

`configure_system_build_properties()` nor the ASan setup script did.

"""

jardondiego requested review from IvanBM18, Xeicker and fernandofloresg May 16, 2026 16:05

Diego Jardon added 6 commits May 16, 2026 16:57

Fix reboot_done tracking to ensure device settings are applied

1390496

Revert previous fix and use accumulated state for reboot_done

e032f7a

Invert logic to needs_reboot and use bitwise OR operator to fix state…

0629dec

… tracking and short-circuit issues

Fix needs_reboot logic so ASan setup clears pending reboots

7c64b50

Fix initialization reboot logic to correctly preserve base requirements

9836c8f

Add unit tests for android device reboot logic

2d75802

jardondiego marked this pull request as ready for review May 17, 2026 00:44

jardondiego requested a review from a team as a code owner May 17, 2026 00:44

IvanBM18 reviewed May 18, 2026

View reviewed changes

IvanBM18 approved these changes May 18, 2026

View reviewed changes

Fix formatting issues from linter in device_test.py

78fd5f9

jardondiego requested a review from letitz May 18, 2026 19:27

Merge branch 'master' into optimize-android-reboots

9bb54d8

fernandofloresg approved these changes May 18, 2026

View reviewed changes

letitz requested changes May 19, 2026

View reviewed changes

		sanitizer_options, sanitizer_options_file_path, wait_for_reboot=False)


		def setup_asan_if_needed():



		def write_data_to_file(contents, file_path):
		def write_data_to_file(contents, file_path, wait_for_reboot=True):

	if needs_full_reboot or not asan_reboot_done:
	if needs_full_reboot and not asan_reboot_done:

	if needs_full_reboot or not asan_reboot_done:
	if not already_rebooted and not asan_reboot_done:

	"""Test that it reboots if build.prop changed, even if ASan didn't run."""
	"""Test that `initialize_device()` reboots the device if
	`configure_system_build_properties()` did, and the ASan setup script did not.

-    """Test that the final reboot is skipped if ASan setup performed a shell restart."""
+    """Test that `initialize_device()` skips calling `reboot()` if
+    `configure_system_build_properties()` did not cause a reboot but the ASan
+    setup script did.
+    """

-    """Test that it still reboots to ensure a clean state if no other steps restarted it."""
+    """Test that `initialize_device()` calls `reboot()` if neither
+    `configure_system_build_properties()` nor the ASan setup script did.
+    """

Conversation

jardondiego commented May 16, 2026

Uh oh!

IvanBM18 May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IvanBM18 left a comment

Choose a reason for hiding this comment

Uh oh!

fernandofloresg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

letitz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

IvanBM18 May 18, 2026 •

edited

Loading